Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

docs: add note to ElementExt::on about dropping the handle #3544

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

lukasschlueter
Copy link

Dropping the RemoveEventHandler returned by ElementExt::on also drops the closure, which invalidates the event listener.

Example code:

let button_ref: NodeRef<Button> = NodeRef::new();

button_ref.on_load(move |btn| {
    let cb = btn.on(leptos::ev::click, move |ev| {
        /* ... */
    });
    // Delay the drop to keep the event listener alive
    on_cleanup(move || std::mem::drop(cb));
});

@gbj
Copy link
Collaborator

gbj commented Feb 7, 2025

Running cargo doc to test this out gives the following error:

warning: unresolved link to `reactive_graph::owner::Owner::on_cleanup`
  --> tachys/src/html/element/element_ext.rs:49:37
   |
49 |     /// Consider using [on_cleanup](reactive_graph::owner::Owner::on_cleanup) to delay dropping:
   |                                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ no item named `reactive_graph` in scope
   |
   = note: `#[warn(rustdoc::broken_intra_doc_links)]` on by default

and a broken link.

This is because the reactive_graph crate is not necessarily included but is an opt-in feature. Perhaps some text similar to the must_use message, or maybe just mentioning "If you're using this with Leptos, you can use on_cleanup"? I never know exactly how to handle these situations where the crate is designed for more general purpose use but there are Leptos-specific docs that would be useful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants